Repository navigation
Conversation
Note: this adjustment relies heavily on the implementation in #1697. Without it, the build will be failing since middlewareUtil.getResolvedFrameworkVersion() does not exists yet
f12aeb1 to
9d59177
Compare
Add getResolvedFrameworkVersion to TRACKED_TASK_UTIL_METHODS and a matching resolvedFrameworkVersion branch in resolveInputValue so that a --framework-version change or workspace toggle between builds causes a cache miss and forces the task to re-run.
9d59177 to
4f0bd4a
Compare
d3xter666
changed the title
| let resolvedFrameworkVersion; | ||
| if (workspace) { | ||
| if (frameworkName === "SAPUI5") { | ||
| resolvedFrameworkVersion = ""; |
There was a problem hiding this comment.
Does having workspace defined here always correspond to the usage of framework libraries from it?
If not, this line would be reached when a ui5-workspace.yaml only contains resolutions for non-framework libraries, which IMHO is not correct.
There was a problem hiding this comment.
Thanks! TBH, I didn't consider external projects for that scenario and now this makes more sense. I have pushed a fix for that with 4f0bd4a
There was a problem hiding this comment.
I wonder whether the updated logic is working as intended in all scenarios.
When the framework is "SAPUI5", but no @sapui5/ projects are in the workspace (e.g. only openui5 is resolved via workspace), the "SAPUI5" version will be set to the version of "sap.ui.core". In case of a -SNAPSHOT version, this is usually correct, but I didn't we decide that we still rather want to omit the SAPUI5 version in such a case, as we don't certainly know it, and do not want to fallback to the OpenUI5 version? I also might remember this incorrectly, so please correct me when I'm wrong here.
| if (frameworkName === "SAPUI5") { | ||
| resolvedFrameworkVersion = ""; | ||
| } else { | ||
| resolvedFrameworkVersion = projectGraph.getProject("sap.ui.core")?.getVersion() ?? ""; |
There was a problem hiding this comment.
This handling here doesn't seem to have the same problem as with SAPUI5, because it will resolve to the correct version regardless of whether sap.ui.core has actually been overridden by a workspace resolution or not. If it was not, this version should equal to the version variable.
There was a problem hiding this comment.
I wonder whether a fallback is even possible here. I mean that is it possible at all to build without "sap.ui.core"?
But if it is possible, then a verssion callback is the correct here and I have fixed it with 4f0bd4a
There are two points listed. Where is 3. ? |
- `if (workspace)` fired even when workspace had no framework libraries (only app-level projects), causing SAPUI5 to resolve to "" instead of the real dist version. Guard tightened to `workspace && Object.keys(providedLibraryMetadata).length > 0`. - SAPUI5/OpenUI5 classification switched from the potentially stale ui5.yaml `frameworkName` to scanning `providedLibraryMetadata` ids for `@sapui5/` — the workspace's actual contribution is the ground truth. - OpenUI5 fallback `?? ""` replaced with `?? version ?? ""`: sap.ui.core is correct whether workspace-overridden or dist-installed, so if it is absent the best-known value is `version`, not silent empty string.
The method was tagged @public but never wired into getInterface, so custom extensions could not reach it. Expose it at Specification Version 5.0, consistent with the other 5.0 TaskUtil accessors (getEnv/getTime/getBuildTime)
| // A workspace only overrides the resolved version when it actually contributed framework | ||
| // libraries; an empty providedLibraryMetadata means ui5-workspace.yaml resolved only | ||
| // non-framework libraries, so the configured/overridden `version` still applies. | ||
| const workspaceContributedFramework = Boolean(workspace) && |
There was a problem hiding this comment.
Is this really an accurate information? It only states that there are libraries in the workspace resolution, but it doesn't mean that they will be used by our graph (e.g. when they are available, but the project doesn't need them).
JIRA: CPOUI5FOUNDATION-839
Relates to: #1681
Introduces
ProjectGraph#setResolvedFrameworkVersion/getResolvedFrameworkVersionto capture the version that actually governs the currentbuildorserverun, as opposed to the value literally written inui5.yaml.Two changes ship together:
1. Resolution logic
Workspace-first: when a workspace is active the configured version and any
--framework-versionoverride are ignored — the workspace controls what is linked. OpenUI5 reports thesap.ui.coreversion from the joined graph; SAPUI5 always stores""because no reliable distribution version is available from the workspace graph.2. Build-cache tracking
getResolvedFrameworkVersionis added toTRACKED_TASK_UTIL_METHODS. When a task reads this value the call is recorded alongside the cached output. On the next build the value is re-derived from the live graph; if it changed (--framework-versionoverride or workspace toggle), the cache misses and the task re-runs. This is prerequisite infrastructure for a follow-up PR that wires the resolved version intogenerateVersionInfoand theversionInfomiddleware (#1681).